Skip to content

Conversation

Sahil-4555
Copy link
Contributor

What type of PR is this?

Other

What does this PR do? Why is it needed?
This PR refactors the ParsePath function to improve performance

Which issues(s) does this PR fix?
This PR addresses inefficiencies in ParsePath that caused extra allocations and slower path resolution. It enhances overall query execution performance within the #15767 feature.

Acknowledgements

@Sahil-4555
Copy link
Contributor Author

@syjn99 — since you contributed to the original ParsePath implementation, I’d really value your feedback on these optimizations. Please let me know if you spot any potential regressions or areas for further improvement.

goos: linux
goarch: amd64
pkg: github.com/OffchainLabs/prysm/v6/encoding/ssz/query
cpu: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
                                       │ changes_bench.txt │          original_bench.txt          │
                                       │      sec/op       │   sec/op     vs base                 │
ParsePath/simple_path-8                        161.6n ± 9%   314.0n ± 5%   +94.37% (p=0.000 n=10)
ParsePath/path_with_leading_dot-8              174.2n ± 6%   327.4n ± 6%   +87.97% (p=0.000 n=10)
ParsePath/nested_arrays-8                      258.5n ± 3%   562.3n ± 2%  +117.52% (p=0.000 n=10)
ParsePath/deep_nested-8                        259.7n ± 3%   484.8n ± 7%   +86.69% (p=0.000 n=10)
ParsePath/path_with_multiple_indices-8         324.1n ± 4%   837.3n ± 4%  +158.33% (p=0.000 n=10)
geomean                                        227.7n        472.2n       +107.35%

                                       │ changes_bench.txt │         original_bench.txt          │
                                       │       B/op        │    B/op     vs base                 │
ParsePath/simple_path-8                         128.0 ± 0%   216.0 ± 0%   +68.75% (p=0.000 n=10)
ParsePath/path_with_leading_dot-8               144.0 ± 0%   232.0 ± 0%   +61.11% (p=0.000 n=10)
ParsePath/nested_arrays-8                       176.0 ± 0%   312.0 ± 0%   +77.27% (p=0.000 n=10)
ParsePath/deep_nested-8                         240.0 ± 0%   456.0 ± 0%   +90.00% (p=0.000 n=10)
ParsePath/path_with_multiple_indices-8          232.0 ± 0%   560.0 ± 0%  +141.38% (p=0.000 n=10)
geomean                                         178.4        331.3        +85.73%

                                       │ changes_bench.txt │          original_bench.txt          │
                                       │     allocs/op     │  allocs/op   vs base                 │
ParsePath/simple_path-8                         2.000 ± 0%    4.000 ± 0%  +100.00% (p=0.000 n=10)
ParsePath/path_with_leading_dot-8               2.000 ± 0%    4.000 ± 0%  +100.00% (p=0.000 n=10)
ParsePath/nested_arrays-8                       4.000 ± 0%    8.000 ± 0%  +100.00% (p=0.000 n=10)
ParsePath/deep_nested-8                         2.000 ± 0%    5.000 ± 0%  +150.00% (p=0.000 n=10)
ParsePath/path_with_multiple_indices-8          5.000 ± 0%   11.000 ± 0%  +120.00% (p=0.000 n=10)
geomean                                         2.759         5.882       +113.15%

Copy link
Contributor

@syjn99 syjn99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for optimizing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants